Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Re-sort inbox list on message sent/received #1846

Merged
merged 6 commits into from
Jun 29, 2023
Merged

Re-sort inbox list on message sent/received #1846

merged 6 commits into from
Jun 29, 2023

Conversation

gdbroman
Copy link
Contributor

@gdbroman gdbroman commented Jun 28, 2023 •
edited
Loading

Ticket: RE-277

Description

  • Re-sort inbox list on message sent/received
  • Fix "Syncing messages" state
  • Add new spinner when chat log is syncing
  • Fix pinned inboxes taking 5s to show up

Reviewer Checklist

  • Pipeline passes
  • Docs have been added / updated
  • Tests have been added / updated
  • Has been refactored if necessary

@vercel
Copy link

vercel bot commented Jun 28, 2023 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

3 Ignored Deployments
Name Status Preview Comments Updated (UTC)
holium-com ⬜️ Ignored (Inspect) Jun 28, 2023 6:04pm
hosting-holium-com ⬜️ Ignored (Inspect) Jun 28, 2023 6:04pm
join-holium-com ⬜️ Ignored (Inspect) Jun 28, 2023 6:04pm

@gdbroman gdbroman marked this pull request as ready for review June 28, 2023 13:10
@gdbroman gdbroman requested a review from drunkplato June 28, 2023 13:10
@gdbroman gdbroman self-assigned this Jun 28, 2023
@gdbroman gdbroman added the bug Something isn't working label Jun 28, 2023
gdbroman
gdbroman commented Jun 28, 2023
View reviewed changes
app/src/renderer/stores/chat.store.ts
const aTimestamp = a.updatedAt || a.metadata.timestamp;
const bTimestamp = b.updatedAt || b.metadata.timestamp;
const aTimestamp =
a.lastMessage?.createdAt || a.updatedAt || a.metadata.timestamp;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The fix was looking at lastMessage.createdAt instead of updatedAt.

@gdbroman
Copy link
Contributor Author

Fixed the "Loading new messages" state

CleanShot.2023-06-28.at.17.00.29.mp4

@gdbroman
Copy link
Contributor Author

Added loading UI to chatlog as well (spinner in top-right corner)

CleanShot.2023-06-28.at.17.47.35.mp4

@gdbroman
Copy link
Contributor Author

@drunkplato what do you think of just showing a pin icon instead of a different background color for pinned chats?

Courier Telegram
CleanShot 2023-06-28 at 20 03 00@2x CleanShot 2023-06-28 at 20 05 03@2x

drunkplato
drunkplato requested changes Jun 29, 2023
View reviewed changes
Copy link
Contributor

@drunkplato drunkplato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This feels great to use. Only one tweak is the spinner on the ChatHeader.

I don't think it should replace the menu button when loading. Instead, the spinner should appear to the left of the button

image

@drunkplato
Copy link
Contributor

@drunkplato what do you think of just showing a pin icon instead of a different background color for pinned chats?

Courier Telegram
CleanShot 2023-06-28 at 20 03 00@2x CleanShot 2023-06-28 at 20 05 03@2x

Let's do it.

@gdbroman
Copy link
Contributor Author

gdbroman commented Jun 29, 2023 •
edited
Loading

I don't think it should replace the menu button when loading. Instead, the spinner should appear to the left of the button

Makes sense, maybe it could be a small position-absolute spinner in the bottom-right corner:

CleanShot 2023-06-29 at 11 57 47@2x

Edit, this looks good enough to me:

CleanShot 2023-06-29 at 12 38 36@2x

@vercel
Copy link

vercel bot commented Jun 29, 2023

@gdbroman is attempting to deploy a commit to the Holium Team on Vercel.

To accomplish this, @gdbroman needs to request access to the Team.

Afterwards, an owner of the Team is required to accept their membership request.

If you're already a member of the respective Vercel Team, make sure that your Personal Vercel Account is connected to your GitHub account.

@gdbroman
Copy link
Contributor Author

Done.

CleanShot 2023-06-29 at 12 45 15@2x

@gdbroman gdbroman requested a review from drunkplato June 29, 2023 10:46
@drunkplato drunkplato merged commit 7880974 into master Jun 29, 2023
@drunkplato drunkplato deleted the RE-277 branch June 29, 2023 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@drunkplato drunkplato drunkplato approved these changes

Assignees

@gdbroman gdbroman

Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gdbroman @drunkplato